Remove the trailing comma in the timeline event box if no place name #28
Conversation
|
Thanks for your PR. I'll try to review this soon. Since you specifically asked for feedback:
I think everything else is fine. Keep in mind that my feedback is just based on what I'm used to. If you contribute to other repos on GitHub, the nuances of the process may be different. You can submit all your improvements in separate PRs (with separate branches of your fork). If they deal with the timeline, I'll probably look at them soon as well. |
|
Yes, I am TJB at the Gramps forum, and (of course) TJBFTV for your repository here. Thanks for the feedback. I did forget to change to my branch at GitHub before I submitted my first PR. I now have a local mentor for git and GitHub-related things, so I expect more generally-expected conventions going forward. I'll try to remember to indicate when a PR submission is complete, not a draft, so you can take a look at it. The FamilyTreeView is a wonderful enhancement to Gramps. I don't doubt that it has taken you a couple of years to get to this stage. Compliments to you, sir! |
|
@TJBFTV |
|
Another accidental close/reopen. Sorry again. |
The event box in the timeline shows a trailing comma after the event date, even when there is no event place.
In the case where an event has no place, self.ftv.get_full_place_name_from_event(event) returns "" instead of None. Of the various ways to fix this, this PR leaves the test for None intact and adds a test for "" to the else block, to allow for potential future changes to self.ftv.get_full_place_name_from_event().
I'm submitting this trivial cosmetic change as a way to learn the ins and outs of PR submission at Github. Feedback on my submission process is appreciated.
Also, I have five other PRs (so far) to submit for bug fixes and an enhancement on the assumed TODO list. All of them are for family_tree_view_timeline.py. Each PR is about a half-dozen lines or less of contiguous code. Would you like me to submit them all at once, or stagger them over time?